Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

ORCA-568: Ignore push builds for dependabot #424

Merged
merged 1 commit into from
Nov 7, 2023

Conversation

danepowell
Copy link
Contributor

@danepowell danepowell commented Aug 22, 2023

Currently when dependabot pushes a commit in a pull request it triggers two jobs :

  1. Firstly, the job for the pull request changes happening due to this commit.
  2. Secondly, the job for the push event in the pull request branch created in origin by dependabot

This PR intends to eliminate the second one i.e. triggering of a job due to a push event in a branch created by dependabot as it is redundant.

@secretsayan secretsayan added the chore Modification related to company packages or other stuffs which do not have a relevant label label Aug 28, 2023
@TravisCarden
Copy link
Contributor

@danepowell, with this change, what would happen if Dependabot rebased a pull request? It seems to me that the updated pull request would not trigger a build. Does that sound right?

@danepowell
Copy link
Contributor Author

A rebase shouldn't be different from any other commit to a pull request in terms of workflow events. It will still trigger a new workflow run. We've been using this on Acquia CLI for over a year and Dependabot still happily does its thing: https://github.com/acquia/cli/blob/main/.github/workflows/ci.yml#L6

@TravisCarden
Copy link
Contributor

Okay, so just to make sure I understand you correctly, @danepowell, with this change, Dependabot PRs and rebases done by Dependabot will both trigger a build? But if a user pushes to a Dependabot PR it won't trigger a build? If I understand properly, the only behavior this would change is that if a human user commits and pushes up a new commit, the CI job won't run and the new commit won't be tested. Why is that desirable?

@danepowell
Copy link
Contributor Author

danepowell commented Aug 31, 2023

No, sorry; any commit (human or bot) will still trigger a build. The difference is this: today, any commit to a PR opened from a Dependabot branch will actually trigger two builds. After this change, such a commit will only trigger a single build. This is due to "pushes" (to a branch on the origin repo) and "pull request changes" being two separate events that aren't mutually exclusive.

An example: normally, developers open pull requests from feature branches on their forks, as I've done for this PR. In that case, GitHub Actions only fires a single event for any commit, e.g. pull_request or pull_request_target. But if you open a PR from a feature branch on origin, as Dependabot does, this fires two events: pull_request (as before) but also push. This is bad because some workflows aren't thread-safe.

So unless you have an unusual workflow with developers working directly on the origin repo, this basically only affects Dependabot.

Sorry, I thought I mentioned this in ORCA-568 but maybe not clearly enough.

@TravisCarden
Copy link
Contributor

This is due to "pushes"... and "pull request changes" being two separate events that aren't mutually exclusive.

Okay, that's key. In that case, this makes sense to me. @secretsayan, I would encourage you to add a brief inline code comment summarizing this discussion.

@secretsayan
Copy link
Contributor

@danepowell : I see that you have edited the orca.yml in the example module, was this intentional or did you mean to change the orca.yml in the root directory instead?

@danepowell
Copy link
Contributor Author

@secretsayan I intended to change the example file since this is what downstream projects (such as my own) will reference.

If you want to update the root orca.yml, that's up to you. Let me know if you want me to add that to this PR.

@secretsayan
Copy link
Contributor

@danepowell thanks for the information. I will add it to the root in a different PR.

@secretsayan secretsayan merged commit 8552fd1 into acquia:develop Nov 7, 2023
24 of 27 checks passed
secretsayan pushed a commit that referenced this pull request Nov 7, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
chore Modification related to company packages or other stuffs which do not have a relevant label
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants